Skip to content

Conversation

lightsing
Copy link
Member

@lightsing lightsing commented Sep 30, 2025

patch crate: https://github.com/scroll-tech/elliptic-curves/tree/ceno/k256-13.4

test_secp256k1_ecrecover, runs 5 ecrecover:

  • with precompile: 3845214 steps
  • without: 65928672 steps

Copy link
Collaborator

@hero78119 hero78119 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one minor issue, other LGTM!

@lightsing lightsing force-pushed the feat/secp256k1-guest branch from 59e641a to 20b947b Compare October 9, 2025 02:13
@lightsing
Copy link
Member Author

lightsing commented Oct 9, 2025

@hero78119 I reconsidering the approach to add secp256k1 support, I prefer to fork & modify k256 crate and use it here now.

see also the sp1 codes:

@kunxian-xia kunxian-xia requested a review from hero78119 October 13, 2025 07:23
Copy link
Collaborator

@hero78119 hero78119 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM!

btw @lightsing could you also left some hints for future works, e.g. how to patch syscall for k256 to accelerate bigint operation? it would help when bigint precompile ready 🙏

@lightsing
Copy link
Member Author

Note that in this version, we don't have bigint sqrt and inv support, so this part of code is removed.
see also: https://github.com/sp1-patches/elliptic-curves/blob/f7d8998e05d8cbcbd8e543eba1030a7385011fa8/k256/src/lib.rs#L164-L213

@lightsing lightsing enabled auto-merge October 14, 2025 07:32
Copy link
Collaborator

@hero78119 hero78119 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lightsing lightsing added this pull request to the merge queue Oct 14, 2025
Merged via the queue into master with commit e31dab3 Oct 14, 2025
4 checks passed
@lightsing lightsing deleted the feat/secp256k1-guest branch October 14, 2025 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants